hooks: defer internal OVS startup until charm has configured identity#168
Conversation
hemanthnakkina
force-pushed
the
fix-delay-ovs-start
branch
from
1c253d6 to
d05f1c1
Compare
hemanthnakkina
requested review from
Copilot and
gboutry
and removed request for
gboutry
There was a problem hiding this comment.
Pull request overview
Defers starting internal OVS services during early configure hook runs until the charm has provided a non-placeholder Keystone endpoint, preventing openstack-hypervisor from pre-creating the system@ovs-system datapath and blocking a concurrently installing microovn.
Changes:
- Add a guard in
configure()to skip_ensure_internal_ovs_services()whilenetwork.ovs-managed-by=autoandidentity.auth-urlis still the default placeholder. - Update existing unit tests to account for the new guard.
- Add new unit tests covering internal OVS startup deferral until identity is configured.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openstack_hypervisor/hooks.py | Adds identity-based guard to defer internal OVS service startup in auto mode. |
| tests/unit/test_hooks.py | Adjusts existing configure-hook tests and adds new tests for the startup guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hemanthnakkina
force-pushed
the
fix-delay-ovs-start
branch
from
d05f1c1 to
123a0ec
Compare
When openstack-hypervisor and microovn are deployed simultaneously,
snapd fires the configure hook immediately after snap install — before
the charm has applied any snap config. At that point
network.ovs-managed-by is still 'auto' and the ovn-chassis plug is not
yet connected, so is_ovs_external() returns False. This caused
_ensure_internal_ovs_services to start ovs-vswitchd, which creates the
system@ovs-system kernel datapath. When microovn subsequently tries to
initialise OVS it finds the datapath already in use and fails.
The same issue affects the second configure hook, triggered when the
charm's own install hook calls `snap set network.ovs-managed-by=auto`,
since the identity URL is still the placeholder at that point too.
Fix: before calling _ensure_internal_ovs_services, check whether the
charm has applied its identity configuration. The guard defers OVS
startup when BOTH identity.auth-url is still the placeholder default
("http://localhost:5000/v3") AND identity.username is unset (None).
Checking both fields ensures the guard still clears correctly for
operators that legitimately run Keystone at http://localhost:5000/v3,
since those deployments will always set identity.username. Once either
condition is no longer true the guard clears and _ensure_internal_ovs_services
runs normally.
The guard is only active when network.ovs-managed-by is 'auto'. When
explicitly set to 'hypervisor' the operator's intent is unambiguous and
ovs-vswitchd is started immediately regardless of identity state.
hemanthnakkina
force-pushed
the
fix-delay-ovs-start
branch
from
123a0ec to
e72c732
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| snap.config.get_options.return_value.get.return_value = hooks.DEFAULT_CONFIG[ | ||
| "identity.auth-url" | ||
| ] | ||
|
|
There was a problem hiding this comment.
In this test, snap.config.get_options.return_value.get.return_value is set to the default auth URL, which means the mocked identity_opts.get() will also return that same string for identity.username. That makes the startup guard evaluate as “configured” even if _OVS_MANAGED_BY were still auto, so the test can pass without actually exercising the intended bypass behavior for OVS_MANAGED_BY_HYPERVISOR. Mock identity_opts.get() with a side_effect that returns the placeholder URL for identity.auth-url and None for identity.username so the guard would defer in auto but is bypassed in hypervisor.
| snap.config.get_options.return_value.get.return_value = hooks.DEFAULT_CONFIG[ | |
| "identity.auth-url" | |
| ] | |
| identity_opts = snap.config.get_options.return_value | |
| def _identity_get_side_effect(option, default=None): | |
| if option == "identity.auth-url": | |
| # Still at the placeholder default. | |
| return hooks.DEFAULT_CONFIG["identity.auth-url"] | |
| if option == "identity.username": | |
| # Username has not been configured yet. | |
| return None | |
| return default | |
| identity_opts.get.side_effect = _identity_get_side_effect |
| placeholder default. In that state ``_ensure_internal_ovs_services`` must be | ||
| skipped to avoid creating ``system@ovs-system`` before microovn installs. |
There was a problem hiding this comment.
This docstring says the snap defers startup by checking only that identity.auth-url equals the placeholder default, but the actual guard in configure() also requires identity.username to be unset. Please update the docstring to reflect the real condition (auth-url placeholder AND username unset) so the test description matches the behavior under test.
| placeholder default. In that state ``_ensure_internal_ovs_services`` must be | |
| skipped to avoid creating ``system@ovs-system`` before microovn installs. | |
| placeholder default *and* that ``identity.username`` is still unset. In that | |
| state ``_ensure_internal_ovs_services`` must be skipped to avoid creating | |
| ``system@ovs-system`` before microovn installs. |
When openstack-hypervisor and microovn are deployed simultaneously, snapd fires the configure hook immediately after snap install — before the charm has applied any snap config. At that point network.ovs-managed-by is still 'auto' and the ovn-chassis plug is not yet connected, so is_ovs_external() returns False. This caused _ensure_internal_ovs_services to start ovs-vswitchd, which creates the system@ovs-system kernel datapath. When microovn subsequently tries to initialise OVS it finds the datapath already in use and fails.
The same issue affects the second configure hook, triggered when the charm's own install hook calls
snap set network.ovs-managed-by=auto, since the identity URL is still the placeholder at that point too.Fix: before calling _ensure_internal_ovs_services, check whether identity.auth-url is still the placeholder default ("http://localhost:5000/v3"). While it is, the charm has not yet established its identity relation and internal OVS startup is deferred. Once the charm sets a real Keystone endpoint the guard clears and _ensure_internal_ovs_services runs normally.
The guard is only active when network.ovs-managed-by is 'auto'. When explicitly set to 'hypervisor' the operator's intent is unambiguous and ovs-vswitchd is started immediately regardless of identity.auth-url.